exclude removed reactables #1750

willmcgugan · 2023-02-09T11:06:44Z

When calling switch_screen a screen is removed, but the watchers in the header were called after the DOM had been deleted.

Deprecates reactive.watch in favor of DOMNode.watch
Removes watch callbacks for detached nodes

davep · 2023-02-09T11:35:02Z

Running the example code in the related issue, with this PR, causes this error when I follow the instructions in that issue:

╭──────────────────────────────────────────────────────────── Traceback (most recent call last) ─────────────────────────────────────────────────────────────╮
│ /Users/davep/temp/textual/src/textual/widgets/_header.py:136 in on_mount                                                                                   │
│                                                                                                                                                            │
│   133 │   │   def set_sub_title(sub_title: str) -> None:                                                                                                   │
│   134 │   │   │   self.query_one(HeaderTitle).sub_text = sub_title                                                                                         │
│   135 │   │                                                                                                                                                │
│ ❱ 136 │   │   watch(self.app, "title", set_title)                                                                                                          │
│   137 │   │   watch(self.app, "sub_title", set_sub_title)                                                                                                  │
│   138                                                                                                                                                      │
│                                                                                                                                                            │
│ ╭───────────────────────────────────── locals ─────────────────────────────────────╮                                                                       │
│ │          self = Header()                                                         │                                                                       │
│ │ set_sub_title = <function Header.on_mount.<locals>.set_sub_title at 0x103fbf2e0> │                                                                       │
│ │     set_title = <function Header.on_mount.<locals>.set_title at 0x103fbf060>     │                                                                       │
│ ╰──────────────────────────────────────────────────────────────────────────────────╯                                                                       │
│                                                                                                                                                            │
│ /Users/davep/temp/textual/src/textual/reactive.py:352 in watch                                                                                             │
│                                                                                                                                                            │
│   349 │   watcher_list.append((obj, callback))                                                                                                             │
│   350 │   if init:                                                                                                                                         │
│   351 │   │   current_value = getattr(obj, attribute_name, None)                                                                                           │
│ ❱ 352 │   │   Reactive._check_watchers(obj, attribute_name, current_value)                                                                                 │
│   353                                                                                                                                                      │
│                                                                                                                                                            │
│ ╭───────────────────────────────────────────────────────────────────── locals ─────────────────────────────────────────────────────────────────────╮       │
│ │ attribute_name = 'title'                                                                                                                         │       │
│ │       callback = <function Header.on_mount.<locals>.set_title at 0x103fbf060>                                                                    │       │
│ │  current_value = 'ModalApp'                                                                                                                      │       │
│ │           init = True                                                                                                                            │       │
│ │            obj = ModalApp(title='ModalApp', classes={'-dark-mode'})                                                                              │       │
│ │   watcher_list = [                                                                                                                               │       │
│ │                  │   (ModalApp(title='ModalApp', classes={'-dark-mode'}), <function Header.on_mount.<locals>.set_title at 0x103f8af20>),         │       │
│ │                  │   (ModalApp(title='ModalApp', classes={'-dark-mode'}), <function Header.on_mount.<locals>.set_title at 0x103fbca40>),         │       │
│ │                  │   (ModalApp(title='ModalApp', classes={'-dark-mode'}), <function Header.on_mount.<locals>.set_title at 0x103fbf060>)          │       │
│ │                  ]                                                                                                                               │       │
│ │       watchers = {                                                                                                                               │       │
│ │                  │   'title': [                                                                                                                  │       │
│ │                  │   │   (ModalApp(title='ModalApp', classes={'-dark-mode'}), <function Header.on_mount.<locals>.set_title at 0x103f8af20>),     │       │
│ │                  │   │   (ModalApp(title='ModalApp', classes={'-dark-mode'}), <function Header.on_mount.<locals>.set_title at 0x103fbca40>),     │       │
│ │                  │   │   (ModalApp(title='ModalApp', classes={'-dark-mode'}), <function Header.on_mount.<locals>.set_title at 0x103fbf060>)      │       │
│ │                  │   ],                                                                                                                          │       │
│ │                  │   'sub_title': [                                                                                                              │       │
│ │                  │   │   (                                                                                                                       │       │
│ │                  │   │   │   ModalApp(title='ModalApp', classes={'-dark-mode'}),                                                                 │       │
│ │                  │   │   │   <function Header.on_mount.<locals>.set_sub_title at 0x103f58e00>                                                    │       │
│ │                  │   │   ),                                                                                                                      │       │
│ │                  │   │   (                                                                                                                       │       │
│ │                  │   │   │   ModalApp(title='ModalApp', classes={'-dark-mode'}),                                                                 │       │
│ │                  │   │   │   <function Header.on_mount.<locals>.set_sub_title at 0x103fbcf40>                                                    │       │
│ │                  │   │   )                                                                                                                       │       │
│ │                  │   ]                                                                                                                           │       │
│ │                  }                                                                                                                               │       │
│ ╰──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯       │
│                                                                                                                                                            │
│ /Users/davep/temp/textual/src/textual/widgets/_header.py:131 in set_title                                                                                  │
│                                                                                                                                                            │
│   128 │                                                                                        ╭────── locals ──────╮                                      │
│   129 │   def on_mount(self) -> None:                                                          │  self = Header()   │                                      │
│   130 │   │   def set_title(title: str) -> None:                                               │ title = 'ModalApp' │                                      │
│ ❱ 131 │   │   │   self.query_one(HeaderTitle).text = title                                     ╰────────────────────╯                                      │
│   132 │   │                                                                                                                                                │
│   133 │   │   def set_sub_title(sub_title: str) -> None:                                                                                                   │
│   134 │   │   │   self.query_one(HeaderTitle).sub_text = sub_title                                                                                         │
│                                                                                                                                                            │
│ /Users/davep/temp/textual/src/textual/dom.py:818 in query_one                                                                                              │
│                                                                                                                                                            │
│   815 │   │   │   query_selector = selector.__name__                                                                                                       │
│   816 │   │   query: DOMQuery[Widget] = DOMQuery(self, filter=query_selector)                                                                              │
│   817 │   │                                                                                                                                                │
│ ❱ 818 │   │   return query.only_one() if expect_type is None else query.only_one(expect_type)                                                              │
│   819 │                                                                                                                                                    │
│   820 │   def set_styles(self, css: str | None = None, **update_styles) -> None:                                                                           │
│   821 │   │   """Set custom styles on this object."""                                                                                                      │
│                                                                                                                                                            │
│ ╭──────────────────────────── locals ────────────────────────────╮                                                                                         │
│ │       DOMQuery = <class 'textual.css.query.DOMQuery'>          │                                                                                         │
│ │    expect_type = None                                          │                                                                                         │
│ │          query = <DOMQuery Header() filter='HeaderTitle'>      │                                                                                         │
│ │ query_selector = 'HeaderTitle'                                 │                                                                                         │
│ │       selector = <class 'textual.widgets._header.HeaderTitle'> │                                                                                         │
│ │           self = Header()                                      │                                                                                         │
│ ╰────────────────────────────────────────────────────────────────╯                                                                                         │
│                                                                                                                                                            │
│ /Users/davep/temp/textual/src/textual/css/query.py:243 in only_one                                                                                         │
│                                                                                                                                                            │
│   240 │   │   """                                                                              ╭──────────────────────── locals ────────────────────────╮  │
│   241 │   │   # Call on first to get the first item. Here we'll use all of the                 │ expect_type = None                                     │  │
│   242 │   │   # testing and checking it provides.                                              │        self = <DOMQuery Header() filter='HeaderTitle'> │  │
│ ❱ 243 │   │   the_one = self.first(expect_type) if expect_type is not None else self.first()   ╰────────────────────────────────────────────────────────╯  │
│   244 │   │   try:                                                                                                                                         │
│   245 │   │   │   # Now see if we can access a subsequent item in the nodes. There                                                                         │
│   246 │   │   │   # should *not* be anything there, so we *should* get an                                                                                  │
│                                                                                                                                                            │
│ /Users/davep/temp/textual/src/textual/css/query.py:214 in first                                                                                            │
│                                                                                                                                                            │
│   211 │   │   │   │   │   )                                                                    ╭──────────────────────── locals ────────────────────────╮  │
│   212 │   │   │   return first                                                                 │ expect_type = None                                     │  │
│   213 │   │   else:                                                                            │        self = <DOMQuery Header() filter='HeaderTitle'> │  │
│ ❱ 214 │   │   │   raise NoMatches(f"No nodes match {self!r}")                                  ╰────────────────────────────────────────────────────────╯  │
│   215 │                                                                                                                                                    │
│   216 │   @overload                                                                                                                                        │
│   217 │   def only_one(self) -> Widget:                                                                                                                    │
╰────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯
NoMatches: No nodes match <DOMQuery Header() filter='HeaderTitle'>

This looks to be the same result as when I tested with main a couple of days ago.

willmcgugan · 2023-02-09T11:42:37Z

Bit premature @davep

davep · 2023-02-09T11:44:10Z

Bit premature @davep

D'oh! Could have sworn I'd seen a review request come in on this one. Apologies, getting stuff mixed up!

willmcgugan · 2023-02-09T11:51:34Z

No worries. Ready now.

davep · 2023-02-09T11:52:21Z

Hey look, it works now! ;-)

davep · 2023-02-09T11:24:21Z

src/textual/message_pump.py

@@ -123,6 +123,20 @@ def log(self) -> Logger:
        """
        return self.app._logger

+    @property
+    def is_attached(self) -> bool:
+        """Check the node is attached to the app via the DOM."""


As a property, perhaps this would be better worded more like "Is the node attached to the app via the DOM?".

Just the type of thing I would say :')

willmcgugan added 2 commits February 9, 2023 11:04

exclude removed reactables

6b91501

test for is_attached

93acc27

Added watch method

392b56e

willmcgugan added 3 commits February 9, 2023 11:45

typing and changelog

85df8d7

Added snapshot

3a9c052

changelog

67e19d8

willmcgugan requested review from darrenburns, davep and rodrigogiraoserrao February 9, 2023 11:51

snapshot

f450d98

davep approved these changes Feb 9, 2023

View reviewed changes

docstring [skip ci]

74fc850

willmcgugan merged commit 67c2127 into main Feb 9, 2023

willmcgugan deleted the fix-screen-switch branch February 9, 2023 12:08

willmcgugan restored the fix-screen-switch branch February 9, 2023 12:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

exclude removed reactables #1750

exclude removed reactables #1750

willmcgugan commented Feb 9, 2023 •

edited

Loading

davep commented Feb 9, 2023

willmcgugan commented Feb 9, 2023

davep commented Feb 9, 2023

willmcgugan commented Feb 9, 2023

davep commented Feb 9, 2023

davep Feb 9, 2023

rodrigogiraoserrao Feb 9, 2023

exclude removed reactables #1750

exclude removed reactables #1750

Conversation

willmcgugan commented Feb 9, 2023 • edited Loading

davep commented Feb 9, 2023

willmcgugan commented Feb 9, 2023

davep commented Feb 9, 2023

willmcgugan commented Feb 9, 2023

davep commented Feb 9, 2023

davep Feb 9, 2023

Choose a reason for hiding this comment

rodrigogiraoserrao Feb 9, 2023

Choose a reason for hiding this comment

willmcgugan commented Feb 9, 2023 •

edited

Loading